-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#50: Added shallow abstraction for boto3 #51
Conversation
…s needed by this project - Moved AwcAccess under module aws
… wrong usage of methods of the mocked class
…ngs left by the split from the timeout PR
Integration test for the shallow abstraction will be implemented with a separate ticket #52. For that reason, please review the boto calls very thoroughly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some feedback.
Note: I am not familiar enough with AWS to reason about the expected AWS behavior and contracts.
exasol_script_languages_container_ci_setup/lib/aws/wrapper/cloudformation_service.py
Outdated
Show resolved
Hide resolved
exasol_script_languages_container_ci_setup/lib/aws/wrapper/codebuild_service.py
Outdated
Show resolved
Hide resolved
exasol_script_languages_container_ci_setup/lib/aws/wrapper/datamodels/codebuild.py
Show resolved
Hide resolved
exasol_script_languages_container_ci_setup/lib/aws/wrapper/datamodels/codebuild.py
Show resolved
Hide resolved
exasol_script_languages_container_ci_setup/lib/aws/wrapper/datamodels/common.py
Outdated
Show resolved
Hide resolved
exasol_script_languages_container_ci_setup/lib/aws/wrapper/datamodels/common.py
Show resolved
Hide resolved
exasol_script_languages_container_ci_setup/lib/aws/wrapper/secretsmanager_service.py
Outdated
Show resolved
Hide resolved
test/unit_tests/aws/wrapper/datamodels/secretsmanager/test_secret.py
Outdated
Show resolved
Hide resolved
test/unit_tests/aws/wrapper/datamodels/secretsmanager/test_secret.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Nicola Coretti <[email protected]>
def test_arn_exists(): | ||
expected = Secret(arn=ARN(aws_arn="EXPECTED_ARN")) | ||
boto_secret = {"ARN": expected.arn.aws_arn} | ||
actual = Secret.from_boto(boto_secret) | ||
assert actual.arn == expected.arn | ||
|
||
|
||
def test_arn_not_exists(): | ||
with pytest.raises(KeyError): | ||
boto_secret = {} | ||
secret = Secret.from_boto(boto_secret) | ||
|
||
|
||
def test_with_extra_keys(): | ||
expected = Secret(arn=ARN(aws_arn="EXPECTED_ARN")) | ||
boto_secret = { | ||
"ARN": expected.arn.aws_arn, | ||
"extra1": None, | ||
"extra2": 1 | ||
} | ||
actual = Secret.from_boto(boto_secret) | ||
assert actual == expected | ||
|
||
|
||
def test_empty_arn(): | ||
with pytest.raises(ValueError, match="ARN was None"): | ||
boto_secret = { | ||
"ARN": None | ||
} | ||
secret = Secret.from_boto(boto_secret) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In cases you expect further tests here, they could be rewritten to be parameterized tests. One group of parameterized test for the exception based test and another for the value based ones.
Fixes #50